fix: Overwrite allowances in special exams instead of creating a new …#38488
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @brianjbuck-wgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Instructor API v2 special-exams allowance endpoints so that granting a new allowance overwrites any existing allowance for the same user+exam (rather than allowing multiple allowance types to accumulate), addressing item 1 in openedx/frontend-app-instructor-dashboard#194.
Changes:
- Add a helper to “add or replace” an allowance by removing existing allowances with different keys for the same user+exam.
- Update single-exam and bulk allowance POST endpoints to use the new replacement behavior and to surface user lookup failures.
- Add tests asserting that granting/bulk-granting an allowance with a different key replaces the previous one.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lms/djangoapps/instructor/views/api_v2.py | Introduces allowance replacement helper and wires it into allowance grant/bulk-grant endpoints. |
| lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py | Adds regression tests validating “replace different key” behavior for single and bulk grants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
brianjbuck-wgu
force-pushed
the
bugfix/194-allowance-replace-on-duplicate
branch
from
3d65538 to
712104b
Compare
|
Can you provide more context in the PR description itself? |
|
@wgu-taylor-payne, I updated the PR description. Hopefully that makes more sense and gives a bit more context. |
wgu-taylor-payne
left a comment
wgu-taylor-payne
left a comment
There was a problem hiding this comment.
Looks good, thanks!
Addresses item 1 of this issue: openedx/frontend-app-instructor-dashboard#194
The
edx-proctoringmodel has a unique constraint on (user,proctored_exam,key) wherekeyis the allowance type. The current implementation of the v2 endpoints respects that unique constraint. However, the v1 Instructor Dashboard only allows a learner to have a single allowance at a time regardless of the type. To match that behavior, this PR does an upsert that only allows a single allowance per learner overwriting a previous allowance if it exists even if its of a differentkey(type).